Add batch span processor benchmarks #3017
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sbandadd
requested review from
anuraaga,
arminru,
bogdandrutu,
carlosalberto,
jkwatson,
Oberon00,
pavolloffay,
thisthat and
tylerbenson
as code owners
sbandadd
force-pushed
the
sbandadd-add-benchmarks
branch
2 times, most recently
from
eab438a to
b96b976
Compare
anuraaga
reviewed
Mar 12, 2021
sdk/trace/src/jmh/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorCpuBenchmark.java
Show resolved
Hide resolved
sdk/trace/src/jmh/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorMetrics.java
Outdated
Show resolved
Hide resolved
|
|
||
| private long getMetric(boolean dropped) { | ||
| String labelValue = String.valueOf(dropped); | ||
| Optional<Long> value = |
There was a problem hiding this comment.
I have a feeling two loops will be very similar code while more readable
There was a problem hiding this comment.
The existing code is two loop and it took me a good amount of time to debug why dropped/exported spans metrics are invalid and got a bit confused on what the two loops are doing. Functional would make it easy to see what are getting filtered and how the data is getting mapped.
That said, I would leave this to the maintainers.
...e/src/jmh/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorMultiThreadBenchmark.java
Outdated
Show resolved
Hide resolved
sbandadd
force-pushed
the
sbandadd-add-benchmarks
branch
2 times, most recently
from
d33590f to
be485ea
Compare
Description: This PR adds two benchmarks. 1. Current benchmark executs forceFlush() on every loop and creates a bottleneck which results in not stressing batch span processor. Current benchmark only measures throughput which is not helpful on its own since number of spans getting exported is also important. BatchSpanProcessorMultiThreadBenchmark is created to address this issue. 2. Measuring CPU usage of exporter thread is also important, but the current benchmarks consumes as much CPU as possible which makes the measurement not meaningful. To maintain a steady state, this PR creates a benchmark that generates 10k spans per second per thread. One would need to attach a profiler such as yourkit or JProfiler to the benchmark run to understand the processor's CPU usage. BatchSpanProcessorCpuBenchmark is created for this purpose. This PR also fixes a bug in calculating number of dropped/export spans. Earlier dropped and exported spans were actually counting the other one. This PR also fixes a big in calculating number of dropped/export spans. Earlier dropped and exported spans were actually counting the other ones.
sbandadd
force-pushed
the
sbandadd-add-benchmarks
branch
from
be485ea to
408915f
Compare
Codecov Report
@@ Coverage Diff @@
## main #3017 +/- ##
===========================================
+ Coverage 0 90.72% +90.72%
- Complexity 0 2813 +2813
===========================================
Files 0 324 +324
Lines 0 8774 +8774
Branches 0 883 +883
===========================================
+ Hits 0 7960 +7960
- Misses 0 551 +551
- Partials 0 263 +263
Continue to review full report at Codecov.
|
anuraaga
approved these changes
Mar 15, 2021
|
I finally got to spend some dedicated time on this today, and got jmh hooked up with the async-profiler profiling option, and I do see some significant overhead introduced by the usage of the ArrayBlockingQueue (although it's been tough to get really consistent results). thanks for putting in the work on this. Now, to agree on the best solution to the issue. :) |
This was referenced Dec 19, 2021
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
This PR adds two benchmarks.
measures throughput which is not helpful on its own since number of spans getting exported is also important. BatchSpanProcessorMultiThreadBenchmark is created to address this issue.
To maintain a steady state, this PR creates a benchmark that generates 10k spans per second per thread. One would need to attach a profiler such as yourkit or JProfiler
to the benchmark run to understand the processor's CPU usage. BatchSpanProcessorCpuBenchmark is created for this purpose.
This PR also fixes a bug in calculating number of dropped/export spans. Earlier dropped and exported spans were actually counting the other one.